don't use f_locals for foreign async generators #3112
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3112 +/- ##
=======================================
Coverage 99.62% 99.63%
=======================================
Files 122 122
Lines 18340 18380 +40
Branches 1222 1223 +1
=======================================
+ Hits 18272 18312 +40
Misses 47 47
Partials 21 21
|
as it's critical that self.foriegn.remove runs
graingert
force-pushed
the
dont-use-f_locals-for-agen-finalization
branch
from
8c4d328
to
fc1f5bb
Compare
…cals-for-agen-finalization
src/trio/_core/_asyncgens.py
Outdated
| try: | ||
| # If the next thing is a yield, this will raise RuntimeError | ||
| # which we allow to propagate | ||
| closer.send(None) |
There was a problem hiding this comment.
actually what do we want to happen here if there's a KI? We want long running finalizers to be interruptible, but then the KeyboardInterrupt exception just gets lost so you need to hit Ctrl+C again to terminate trio
There was a problem hiding this comment.
Where does the KI get lost?
If it's in this closer after we call .send(None), doesn't it get raised here and will propagate up? I assume propagation here is fine because we already have a RuntimeError being raised here.
(I'm basing my assumptions off how generators behave, I don't know anything about async generator cleanup...)
There was a problem hiding this comment.
The finalizer gets called by tp_finalize which is called by the GC which catches the exception
There was a problem hiding this comment.
I see... could we catch the KeyboardInterrupt and basically tell trio that it got another Ctrl+C? It seems to be raised normally:
>>> trio.run(main)
Exception ignored in: <async_generator object awaits_after_yield at 0x000001EF4C0F1F00>
Traceback (most recent call last):
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_asyncgens.py", line 120, in finalizer
closer.send(None)
File "<stdin>", line 6, in awaits_after_yield
KeyboardInterrupt:(I also got this great error message in the REPL while playing around with this...)
Funny error message I ran into in the REPL
Exception ignored in: <function Nursery.__del__ at 0x000001EF4BCC54E0>
Traceback (most recent call last):
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_run.py", line 1372, in __del__
assert not self._children
^^^^^^^^^^^^^^^^^^
AssertionError:
Exception ignored in: <coroutine object Runner.init at 0x000001EF4C0239A0>
Traceback (most recent call last):
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_run.py", line 2030, in init
async with open_nursery() as run_sync_soon_nursery:
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_run.py", line 1030, in __aexit__
new_exc = await self._nursery._nested_child_finished(exc)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_run.py", line 1192, in _nested_child_finished
self._add_exc(nested_child_exc)
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_run.py", line 1170, in _add_exc
self.cancel_scope.cancel()
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_ki.py", line 183, in wrapper
return fn(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_run.py", line 870, in cancel
self._cancel_status.recalculate()
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_run.py", line 459, in recalculate
task._attempt_delivery_of_any_pending_cancel()
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_run.py", line 1545, in _attempt_delivery_of_any_pending_cancel
self._attempt_abort(raise_cancel)
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_run.py", line 1527, in _attempt_abort
success = self._abort_func(raise_cancel)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_io_windows.py", line 740, in abort_fn
self._refresh_afd(base_handle)
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_io_windows.py", line 653, in _refresh_afd
_check(
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_io_windows.py", line 301, in _check
raise_winerror()
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_windows_cffi.py", line 520, in raise_winerror
raise OSError(0, msg, filename, winerror, filename2)
OSError: [WinError 6] The handle is invalid
Exception ignored in: <function Nursery.__del__ at 0x000001EF4BCC54E0>
Traceback (most recent call last):
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_run.py", line 1372, in __del__
assert not self._children
^^^^^^^^^^^^^^^^^^
AssertionError:
Exception ignored in: <function Nursery.__del__ at 0x000001EF4BCC54E0>
Traceback (most recent call last):
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_run.py", line 1372, in __del__
assert not self._children
^^^^^^^^^^^^^^^^^^
AssertionError:
Exception ignored in: <async_generator object awaits_after_yield at 0x000001EF4C0F0040>
Traceback (most recent call last):
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_asyncgens.py", line 88, in finalizer
runner.entry_queue.run_sync_soon(
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_entry_queue.py", line 145, in run_sync_soon
self.wakeup.wakeup_thread_and_signal_safe()
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_wakeup_socketpair.py", line 39, in wakeup_thread_and_signal_safe
self.write_sock.send(b"\x00")
OSError: [WinError 10038] An operation was attempted on something that is not a socket
Exception ignored in: <coroutine object NurseryManager.__aexit__ at 0x000001EF4C01B120>
Traceback (most recent call last):
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_run.py", line 1030, in __aexit__
new_exc = await self._nursery._nested_child_finished(exc)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\A5rocks\Documents\trio\src\trio\_core\_run.py", line 1220, in _nested_child_finished
assert popped is self
^^^^^^^^^^^^^^
AssertionError:
Not the most clear :^)
There was a problem hiding this comment.
It seems to be raised normally
But the message says the exception is ignored?
There was a problem hiding this comment.
Maybe we could call signal.raise_signal(signal.SIGINT) in the except KeyboardInterrupt: and call it a day? Not a great solution IMO. (though actually that ... would probably raise in that except statement. ouch? nevermind)
There was a problem hiding this comment.
Hmm. The except KeyboardInterrupt would have to be outside the _finalize_without_ki_protection
There was a problem hiding this comment.
How about this awful hack?:
except KeyboardInterrupt:
@enable_ki_protection
def killer():
signal.raise_signal(signal.SIGINT)
try:
killer()
except KeyboardInterrupt:
# turns out the trio KI handler isn't installed. Nothing good to do
raise KeyboardInterrupt from Noneedit: sorry, your comments weren't showing.
There was a problem hiding this comment.
I think what I'm going to do here is put the KI protection on all the critical and guaranteed fast trio code and unprotect any of the host loop stuff. Then we can survey someone who uses guest mode and KI and async generators without aclose and find out what they want
There was a problem hiding this comment.
How about this awful hack?:
except KeyboardInterrupt: @enable_ki_protection def killer(): signal.raise_signal(signal.SIGINT) try: killer() except KeyboardInterrupt: # turns out the trio KI handler isn't installed. Nothing good to do raise KeyboardInterrupt from None
Another KI could arrive while constructing or decorating or load_fast-ing the killer function
This reverts commit 9f1823a.
|
Oh the pypy cache failures are back :( I tried deleting the relevant caches from the GH actions cache page last time this happened but I guess they came back. That still works to fix it temporarily though. (Or they get fixed after a few hours for some reason) |
Fixes #2454